Skip to content

Conversation

@marlewe
Copy link
Contributor

@marlewe marlewe commented Jun 5, 2024

Trying out some approaches for linting the code.

  • First tried prettier-maven-plugin with it's simple configuration. But it lacks an import ordering feature. That feature is an extra plugin for prettier and not yet realized for maven (seems so to me).
  • Tried the (eclipse) formatter-maven-plugin as next, but it also has no possibility for ordering imports
  • The spotless-maven-plugin unites the eclipse formatter and the eclipse import ordering feature. So this seems the best approach for now.

Like described in the README.md, the local build will automatically change the code to fulfil the guidelines. The CI build will only make a check and fails if the code style violates the defined guidelines.

@marlewe marlewe self-assigned this Jun 6, 2024
@marlewe marlewe changed the title Added prettier maven plugin Adding linter Jun 12, 2024
@marlewe
Copy link
Contributor Author

marlewe commented Jun 12, 2024

The code is not yet linted, because we should speak about the specific guidelines we'd like to define. That's why the CI build fails, currently.

@RKrahl
Copy link
Member

RKrahl commented Jul 31, 2024

Many thanks for submitting this! Although I'm not able to formally start a review as long as the PR is marked as draft, I might just add a preliminary one as a comment here:

First of all, generally, this looks good to me. There are a few minor remarks, though:

  • Obviously, as you noted yourself in your last comment, we need to discuss the style guidelines being imposed. And we definitely should include @GonozalIX in that discussion.
  • If this is supported by the spotless plugin, we might consider to downgrade a few of the guidelines to warnings, e.g. spotless should emit a warning for a violation of the particular rule, but the CI should not fail on this and a local run should not modify the file. This seems particularly relevant for any limitation of the length of code lines.
  • You created this PR based on the v3_FileInfoImprovementAndRequestHandler branch and thus it implicitly depends on V3 file info improvement and request handler #157. There is no reason for this dependency. In fact, there is no reason for this change to wait for version 3. It could go into master right away instead. (Since you can't change the source branch of a PR in GitHub, this would require to create a new PR, though.)
  • The config files eclipse-formatter.xml and eclipse-importorder.txt are cluttering the main directory. I would prefer them to be stashed away in a subdirectory like .spotless.

@RKrahl RKrahl mentioned this pull request Aug 15, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants